eval build.jl files in a separate Julia process for Pkg.build #13506
Conversation
| end | ||
| """ | ||
| io, pobj = open(detach(`$(Base.julia_cmd()) | ||
| --startup-file=no --history-file=no |
There was a problem hiding this comment.
there might be some env vars or other settings used to customize build behavior that people might put in their juliarc...
There was a problem hiding this comment.
That's true, although I generally think it's a good idea for packages to "remember" the settings from the last build (see how IJulia handles this with a deps/JUPYTER file) so that things don't change unexpectedly on Pkg.update() if the user forgot to set an environment variable the same way as when they added the package...
There was a problem hiding this comment.
You're probably right there. Socially-enforced/demonstrated best practices will hopefully get us there over time, maybe keep in mind and suggest something more built-in and uniform for future revisions of Pkg.
|
@wildart, @StefanKarpinski, looks good to you? |
|
lgtm |
eval build.jl files in a separate Julia process for Pkg.build
|
@tkelman, okay to backport? |
|
(Once we've kicked the tires on this in 0.5 a bit.) |
|
Would like to be cautious on this one. Have you tested it at all on windows yet? I don't think the pkg tests exercise build. |
|
No, I haven't tried it on Windows; I tested it locally on my MacOS machine with both successful and failing builds, but I didn't realize that the Pkg tests don't exercise |
|
I agree that it is not something that should be backported right away, but it is hopefully something that we can backport eventually once it has gotten more testing by 0.5 developers. Otherwise I think we will see a stream of people complaining that |
|
Yeah, winrpm started segfaulting and I suspect it was this PR: https://ci.appveyor.com/project/tkelman/lightxml-jl/branch/master |
|
Local testing points to this as responsible for JuliaPackaging/Homebrew.jl#98 |
|
I just tried |
|
Hmm, probably we need to pass the load path etc. to the subprocess as in the |
|
The WinRPM issue may have been resolved by something else between failing at 03f28fd (https://ci.appveyor.com/project/tkelman/lightxml-jl/build/1.0.116/job/mxfy3r046ty2dyc6) and passing at 8a86270 (https://ci.appveyor.com/project/tkelman/lightxml-jl/build/1.0.117/job/i7mgacf6jx78s2ou). Worth looking into what it was (edit: #13553 was the fix, apparently - will try to verify the initial cause) @malmaud can you test if #13689 fixes the homebrew issue locally? |
|
Not quite sure why, but just switching Homebrew to use |
|
And I tracked down the WinRPM issue to having been caused by #13491, which makes sense given that #13553 was the fix. d3af973 worked https://ci.appveyor.com/project/tkelman/lightxml-jl/build/1.0.126/job/22w3m10o2tdu806w, but 90bce78 failed https://ci.appveyor.com/project/tkelman/lightxml-jl/build/1.0.126/job/8c7j76tfvl0q8rlj. There's a very small chance it was actually #13498 that caused the problem, but I doubt it. I feel like the Homebrew problem is indicative of a bug somewhere in base that should be tracked down, but at least for now this looks safe again. Sorry for blaming the wrong PR! |
This fixes #13458.
It supersedes #13499, which spawned a separate process for each
build.jlfile. Instead, as requested by @tkelman (since launchingjuliais slow on Windows), my new version spawns a single Julia process which evals all thebuild.jlfiles. This should suffice to fix the problems described in #13458 — it isolates the build from the running Julia process, and since packages are built in dependency order it should be okay that their builds share a process.